Merged
Conversation
Coverage ReportFor commit 691014b Click to expand Coverage Report Name Stmts Miss Branch BrPart Cover
--------------------------------------------------------------------------------
+ src/docbuild/models/deliverable.py 180 1 22 0 99.5%
+ src/docbuild/cli/cmd_check/process.py 58 0 22 1 98.8%
+ src/docbuild/models/manifest.py 111 1 12 1 98.4%
+ src/docbuild/utils/pidlock.py 79 1 14 1 97.8%
+ src/docbuild/cli/cmd_cli.py 96 1 10 2 97.2%
+ src/docbuild/cli/cmd_validate/process.py 178 5 52 4 96.1%
+ src/docbuild/cli/callback.py 35 0 10 2 95.6%
+ src/docbuild/utils/concurrency.py 69 3 18 1 95.4%
- src/docbuild/cli/cmd_config/__init__.py 9 1 0 0 88.9%
- src/docbuild/config/xml/stitch.py 47 5 12 0 88.1%
- src/docbuild/cli/cmd_metadata/metaprocess.py 215 26 66 13 82.6%
- src/docbuild/cli/cmd_check/__init__.py 18 5 2 0 65.0%
- src/docbuild/cli/cmd_build/__init__.py 13 5 0 0 61.5%
- src/docbuild/cli/cmd_metadata/__init__.py 27 10 2 0 58.6%
- src/docbuild/cli/cmd_config/environment.py 11 6 2 0 38.5%
--------------------------------------------------------------------------------
+ TOTAL 2992 70 706 25 96.9%
46 files skipped due to complete coverage. |
Collaborator
Thanks for the awesome work Toms. It looks great. I have also added my suggestions. |
sushant-suse
requested changes
Mar 2, 2026
sushant-suse
requested changes
Mar 2, 2026
* Rename function to `process_unordered` * Remove Result, Success, and Failure * For the result, just add the result from the worker or the exception.
For run_parallel
to ensure backpressure propagates through the whole pipeline
The intention is to pass *worker_args and **worker_kwargs to the worker_fn function. We need to explicity tell the type checker that worker_fn is expected to accept the arguments from P in addition to the standard item of type T. We use typing.Concatenate.
tomschr
force-pushed
the
toms/191-producer-consumer
branch
from
e1fb882 to
fb6738e
Compare
Signed-off-by: sushant-suse <[email protected]>
Signed-off-by: sushant-suse <[email protected]>
Signed-off-by: sushant-suse <[email protected]>
tomschr
commented
Apr 13, 2026
Contributor
Author
tomschr
left a comment
tomschr
left a comment
There was a problem hiding this comment.
Ready to be squash-merged! 👍 🎉
Signed-off-by: sushant-suse <[email protected]>
tomschr
force-pushed
the
toms/191-producer-consumer
branch
from
77c6f79 to
691014b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR plays around with the producer-consumer model requested in issue #191. It maybe be integrated or it goes nowhere.
To test it, use these commands:
source devel/activate-aliases.sh upython -m docbuild.utils.concurrencyQuestions:
The current implementation usesTaskGroups, which is considered as the go-to way. Semaphores seems not always that reliable or appropriate for this use case.=> We use
TaskGroupsWhat do we want to "return" inside theconsumer()loop?The consumer calls the worker function. Depending on if it's successful or not, we add either a
Successobject or aFailureobject.I'm not sure if this is a bit too much and can be simplified.
=> We return the respective object and if something fails, a
TaskFailedErrorexception.process_unordered. Possible better names:execute_concurrentlyrun_in_parallel